Skip to content

ref(feedback): Replace react-virtualized with @tanstack/react-virtual in Feedback #95398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jul 15, 2025
Merged

ref(feedback): Replace react-virtualized with @tanstack/react-virtual in Feedback #95398

merged 5 commits into from
Jul 15, 2025
+266 −218

Conversation

ryan953
Copy link
Member

@ryan953 ryan953 commented Jul 12, 2025 •
edited
Loading

The list still looks the same, this was surprisingly easy actually.

SCR-20250712-kygo

2 helper files i pulled in from sentry-toolbar (InfiniteListItems, InfiniteListState) made it easier too, they wrap a bunch of state conditions and also wrap the load-more triggers which really cleanup this <FeedbackList> component now.

Related to https://github.com/getsentry/frontend-tsc/issues/91

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 12, 2025
@ryan953 ryan953 force-pushed the ryan953/partial-focus-fix-conflicts-with-open-pr branch from 75f9749 to b6c6c3a Compare July 12, 2025 18:45
ryan953
ryan953 commented Jul 12, 2025
View reviewed changes
static/app/components/feedback/list/feedbackList.tsx
flex-grow: 1;
min-height: 300px;
padding-bottom: ${space(0.5)};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to add spacing under the last item shown

Image

ryan953
ryan953 commented Jul 12, 2025
View reviewed changes
static/app/components/feedback/list/feedbackList.tsx
Comment on lines +94 to 96
const Centered = styled('div')`
justify-self: center;
`;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No more absolute positioning for the 'loading more' spinner

Before
Image

After
Image

ryan953
ryan953 commented Jul 12, 2025
View reviewed changes
static/app/components/feedback/list/feedbackList.tsx
Comment on lines -178 to -180
position: relative;
top: 50%;
transform: translateY(-50%);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't needed anymore

Image

@ryan953
ref(feedback): Replace react-virtualized with
e35a094 
`@tanstack/react-virtual` in Feedback
@ryan953 ryan953 force-pushed the ryan953/partial-focus-fix-conflicts-with-open-pr branch from b6c6c3a to e35a094 Compare July 12, 2025 19:18
@ryan953 ryan953 marked this pull request as ready for review July 14, 2025 15:40
@ryan953 ryan953 requested a review from a team as a code owner July 14, 2025 15:40
cursor[bot]

This comment was marked as outdated.

Sign in to view

cursor[bot]

This comment was marked as outdated.

Sign in to view

billyvg
billyvg approved these changes Jul 15, 2025
View reviewed changes
@ryan953 ryan953 enabled auto-merge (squash) July 15, 2025 17:25
@ryan953 ryan953 merged commit 374303a into master Jul 15, 2025
46 checks passed
@ryan953 ryan953 deleted the ryan953/partial-focus-fix-conflicts-with-open-pr branch July 15, 2025 17:33
cursor[bot]
cursor bot reviewed Jul 15, 2025
View reviewed changes
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Duplicate Items in UI After Data Processing Change

The loadedRows array, built from data.pages.flatMap(d => d[0]), no longer de-duplicates items. This re-introduces duplicate entries in the UI when the API returns the same item across multiple pages (a scenario previously handled by uniqueItems). Consequently, users see repeated list items and experience inconsistent selection behavior, as the underlying selection logic still uses a de-duplicated list.

static/app/components/infiniteList/infiniteListItems.tsx#L40-L52

sentry/static/app/components/infiniteList/infiniteListItems.tsx

Lines 40 to 52 in 87bfa47

overscan,
queryResult,
}: Props<Data>) {
const {data, hasNextPage, isFetchingNextPage, fetchNextPage} = queryResult;
const loadedRows = data ? data.pages.flatMap(d => d[0]) : [];
const parentRef = useRef<HTMLDivElement>(null);
const rowVirtualizer = useVirtualizer({
count: hasNextPage ? loadedRows.length + 1 : loadedRows.length,
getScrollElement: () => parentRef.current,
estimateSize: estimateSize ?? (() => 100),
overscan: overscan ?? 5,
});

Fix in Cursor • Fix in Web

Was this report helpful? Give feedback by reacting with 👍 or 👎

@ryan953 ryan953 mentioned this pull request Jul 15, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@billyvg billyvg billyvg approved these changes

Assignees
No one assigned
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ryan953 @billyvg